-
Couldn't load subscription status.
- Fork 2k
fix: ucfirst all cookie samesite values #9564
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ucfirst all cookie samesite values #9564
Conversation
|
Yes, this should go to the 4.7 branch. |
|
Oh, we should also adjust the user guide: https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/libraries/cookies/006.php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entry?
|
Thanks, added |
e73a328 to
2cc7775
Compare
|
Thanks all for the reviews. |
Description
If you run
vendor/bin/phpunit --filter CookieTestyou get:This is because when you pass an empty defaults array (or did not pass anything) to
Cookie::setDefaults(), the samesite value set isself::SAMESITE_LAXequal tolax. Now, we transform the samesite value toucfirstin the constructor so that it would be consistent everywhere. Even the cookie config's allowed value for samesite is in ucfirst. I thought we have done that pretty much everywhere but found out this case wasn't. This failure was not caught since the test case is run along with the other tests. Running it by itself shows the issue.I have targeted
4.7since I'm changing the values of the constants so I think this is a behavior change. Let me know if this is ok indevelop.Checklist: